-
Notifications
You must be signed in to change notification settings - Fork 86
Implement index-dump executable to dump unit and record files - #264 #266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
ahoppen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Left a few general comments inline.
Thanks for the review! On it :) |
790786c to
e4a8f81
Compare
|
I have updated the implementation to address all feedback: 1- CLI Infrastructure: Migrated to swift-argument-parser for a more robust command-line interface and auto-generated help pages. Verified the build and functionality locally. Ready for another look! |
|
On it sir :) |
b729de4 to
590e4cd
Compare
|
I have updated the branch to address all your feedback. Here are the key changes:
All 45 tests are passing. Ready for another look! |
ahoppen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple more comments inline.
|
I have an exam in 2 days, so I will be back on January 7th to address the remaining feedback. Thank you so much for your patience and guidance! |
|
I have made changes based on the review you have gave to me |
ahoppen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few more comments inline. I think we’re getting a lot closer 👍🏽
|
On it :) |
ceaa80f to
032d295
Compare
ahoppen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more comments inline. Also, there’s a merge conflict now. Could you resolve this by rebasing your changes on top of the latest main and then also running swift-format -ipr . again?
|
On it boss:) |
|
Hmm, I cannot tell if that’s correct or not. Do you want to push your changes so I can check? We can always revert to a previous commit hash. |
yes , please check ! |
|
OK, that does indeed look messed up. The problem is that you didn’t properly rebase the changes on top of # Remove your formatting commit
git reset --hard HEAD^
# Add remote to the upstream repo in swiftlang if you don’t have it yet
git remote add swiftlang https://github.com/swiftlang/indexstore-db.git
git fetch swiftlang
# Rebase changes on top of swiftlang/indexstore-db’s main branch
git rebase swiftlang/main
# Resolve merge conflicts and finish the rebase
# Run swift-format
swift format -ipr .
# Only commit the changes to the files you have touch, importantly don’t modify files in `INPUTS` or the generated test files.
# Force push your changes
git push -f |
0c18ea9 to
d4aa158
Compare
|
Thank you so much sir
Huge thanks for the git help! The PR is now clean and rebased. I'm very happy to have this back on track! 😊 |
79e7080 to
7e18248
Compare
ahoppen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR doesn’t build, nor does it pass the tests you added. Could you please ensure that it compiles and passes the tests before putting this up for review again?
|
Thank you for the detailed suggestions and for providing the direct code fixes. I am incredibly sorry for the oversights; I have been struggling to focus as I’m in the middle of my university exams. I truly appreciate your patience. I have my second-to-last exam tomorrow and will return to fix the remaining issues with my full focus on the 13th. I am deeply committed to this and will ensure it's resolved then! |
Good luck with your exams. And don’t feel like you have to push this forward during your exams. Feel free to set this aside until you have enough spare time. |
Co-authored-by: Alex Hoppen <[email protected]>
Co-authored-by: Alex Hoppen <[email protected]>
Co-authored-by: Alex Hoppen <[email protected]>
Co-authored-by: Alex Hoppen <[email protected]>
Trying new PR style.
Status: ✅ Ready for Review
Explanation:
Implements a new executable index-dump that allows dumping Unit and Record files from the Index Store using the IndexStore Swift library. This serves as a native Swift replacement for c-index-test (from LLVM), making it significantly easier to debug and inspect Index Store contents (modules, dependencies, symbols, occurrences) without needing to build the full LLVM project.
Scope:
This is Additive. It introduces a new executable target index-dump in Package.swift and does not modify existing IndexStoreDB logic.
Issues:
Fixes: Implement an executable to dump unit and record files #264
Risk:
Low/None. This is a standalone developer tool. It has no impact on the production runtime or the library itself.
Testing:
Verified manually by building the tool and running it against a local .build index store.
Verified unit mode:
1- Correctly lists Module, Main File, and Dependencies.
2- Verified record mode: Correctly lists Symbols and Occurrences.
(Please see attached screenshot for verification)

Reviewers: Suggested by @ahoppen . Open to review by anyone.